Skip to content

Conversation

aaronjeline
Copy link
Collaborator

Changes and re-organizes the Root Cause Analysis to aid in readability and pefromance

@aaronjeline
Copy link
Collaborator Author

One question here is "Is the root cause analysis code enough to move to a separate file?"

@kyleheadley
Copy link
Member

This looks good as far as my understanding goes. Code it clean with comments, algorithms are reasonable and I assume it works and meets your goals. I vote yes for a separate file, but it's not a big deal.

I'm sorry I wasn't able to take part in the discussion earlier, but if this is critical performance and set operations are holding it back, I can suggest not using them. You can add data directly on the graph nodes (or rebuild the graph entirely for this purpose), compute a spanning tree (nearly linear with modern algorithms) or whatever is necessary for cycle elimination, and then collect data in one pass. If the cycles are handled well you may not even need to de-dup, and can use append-only arrays. Was this considered? How does current performance scale?

Even if the above works, this update is fine to include in the mean time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants